Skip to content

feat(hooks): enforce the mechanical rules with PreToolUse hooks - #553

Draft
kixelated wants to merge 11 commits into
mainfrom
quest/m0/token-waste/rule-checks
Draft

kixelated wants to merge 11 commits into
mainfrom
quest/m0/token-waste/rule-checks

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/token-waste/rule-checks.md: the rules in assets/CONTEXT.md that a program can check are now enforced by a PreToolUse hook instead of read as prose, for Claude Code and Codex, and quest setup gives Codex the same SessionStart rules hook Claude Code got in moq-dev/quest#517.

Behavior

quest setup installs one more entry per agent, each running quest hook:

  • Claude Code (settings.json): PreToolUse on Bash|Write|Edit|MultiEdit, next to the existing SessionStart and SubagentStart.
  • Codex ($CODEX_HOME/hooks.json, written only when that directory exists): SessionStart (startup|resume|clear|compact) with additionalContextLimit: 10000, and PreToolUse on Bash|apply_patch. quest setup prints a reminder to trust them through /hooks.

In a repository with the Quests: line, quest hook denies a tool call, giving the rule as the reason, when it:

  • adds an em dash in an edit, a written file, a commit message, or a new GitHub post, counted against the text it replaces and skipping > quote lines;
  • runs git push with a refspec lacking : (or none), outside --delete, --tags, --all, --branches, --mirror;
  • force-pushes (-f in any cluster, --force, --force-with-lease, --mirror, a + refspec), or runs git rebase or git pull --rebase, unless the command starts with QUEST_USER_ASKED=1; git rebase --abort/--quit pass;
  • posts to GitHub (gh pr create|comment|review, gh issue create|comment, gh api with a body field, not PATCH/PUT) with a body lacking a (Written by ...) line;
  • writes scratch under /tmp outside the checkout: redirections, tee, mkdir, touch, the destination of cp/mv/ln/rsync, git clone, git worktree add, mktemp without a directory, and Write/Edit/apply_patch paths.

A new src/cli/shell.ts splits a command line into simple commands (quotes, &&/||/;/pipes/subshells, redirections, here-documents and here-strings), and the checks peel env, assignments, command, timeout, direnv exec, nix develop --command, quest gate, and bash -c to reach the real command. The footer and em-dash checks read a body only from the command line itself: --body, a -f body= field, git commit -m, and --body-file - with a here-document. They never read a body file from disk.

The checked prose left assets/CONTEXT.md (9,997 to 9,535 characters). quest context and the runner's prompt still print the rules, as a last ## Checked section built from the same strings the hook denies with, so an agent without the hook, or a kix runner job, still reads them; only the hook's context leaves it out.

What stayed prose, and why

  • "Never write them elsewhere in or beside the checkout, or in a directory other agents share": a write outside the checkout cannot be told apart from legitimate work, so only /tmp is checked.
  • When to merge the base (only on a conflict or a landing refusal): a judgment, not visible in one command.
  • Em dashes in the agent's replies to the user: no tool call to check. Removed from the always-loaded prose with the rest, as the quest decided.
  • The footer and em-dash rules for a body posted from a file (--body-file path, --body-file - < path, -F body=@path, git commit -F path) or piped into gh: passes unread, since the file may hold something else by the time the line runs, and every review finding in six rounds came from guessing it. assets/CONTEXT.md keeps a line next to "Pass PR bodies to gh on stdin" saying such a body goes unchecked and still needs the footer and no em dashes.

Choices made without asking

  • Force-push and rebase: a check cannot know whether the user asked, so the denial says to prefix QUEST_USER_ASKED=1 when the user asked in their own words. Rejected: keeping them prose (the habitual mistake goes unchecked), and Claude Code's ask decision (Codex does not support it and lets the call through, and it stalls unattended sessions).
  • Em dashes count against the replaced text, so editing a file that already holds them passes, and > quote lines keep quoted ones. gh pr edit, gh issue edit, and gh api -X PATCH are not checked for em dashes or the footer, since an existing body holds other authors' text, such as a reviewer's record.
  • The footer is a last line matching (Written by ...), case-insensitive, with only whitespace after it. A section quest review appends later never trips it, since that is an edit.
  • The checks act only where quest hook already adds the context: repositories with the Quests: line.
  • /tmp paths inside the checkout holding the cwd pass, so a checkout that lives under /tmp keeps working. A $TMPDIR path passes, since TMPDIR may name the checkout (a kix runner job sets it to the job's checkout); mktemp without a directory still counts as /tmp unless a TMPDIR= assignment in front of it says otherwise.
  • Help and dry-run modes (git <sub> --help, mktemp --help/-u) pass, as do the read-only operands of cp/mv/ln -t <dir> and touch -r <file>.
  • Codex gets no SubagentStart entry; the quest's Plan asked only for SessionStart parity.
  • Commit messages with a Co-Authored-By: trailer were exempt from the footer; the hook never checks commits for it, so that clause went with the rule.

Codex evidence

  • additionalContextLimit is in tokens: a hook printing 10,000 characters of word (about 2,000 tokens) spilled to hook_outputs at a limit of 1000, but not at 3000 or the default. The limit is set to the rules' 10,000-character cap, which bounds their tokens.
  • quest review keeps user hooks out: in this checkout, codex exec ran the user's trusted SessionStart hook (hook: SessionStart in its output, its .direnv/codex-env.sh written), and codex exec --ignore-user-config did not (no output line, no file). With --dangerously-bypass-hook-trust, hooks.json does load under --ignore-user-config, so what keeps hooks out of reviews is that their trust lives in config.toml's [hooks.state], which the flag skips. No change to quest review was needed.

Public API

  • quest setup also writes $CODEX_HOME/hooks.json (or ~/.codex/hooks.json) when that directory exists, and adds the PreToolUse entry to Claude Code's settings. Rerunning it replaces the earlier entries.
  • quest hook handles PreToolUse input from either agent and prints a permissionDecision: "deny" on a broken rule.
  • quest context ends with a ## Checked section.
  • Wire: none.

Validation

  • just check and just test pass (1228 + 562 tests).
  • New tests: test/rules.test.ts (parser, each rule's blocked and passing cases) and test/hooks.test.ts (Codex target, handler settings, PreToolUse dispatch).
  • Live in Claude Code: with the built hook in this checkout's ignored .claude/settings.local.json, git push --dry-run origin main was denied with the refspec rule. A quest hook call takes about 0.1s.

Decisions

  • Design checkpoint after review round 3 (three rounds of findings, all in how the gh check finds a post's body; the open one read a body file rewritten by an expanding here-document from its stale contents on disk), settled by the maintainer on 2026-11-08 in moq-dev/quest#553 (comment): ✅ narrow the body sources: drop the in-command file tracking, so a body file the same command line writes (by redirection, tee, or as the destination of cp/mv/ln/rsync) reads as unknown and passes, while --body-file - with a here-document stays checked directly (rejected: keep in-command tracking and mark expanded writes as unknown; merge as is; drop the footer and em-dash checks on gh commands).
  • Second design checkpoint after review round 3 of the second session (scores 6, 10, 9; the findings after the first pick all came from reading body files from disk, first after a cd on the line, then inside a nested shell after one), settled by the maintainer on 2026-11-08 in moq-dev/quest#553 (comment): ✅ check inline bodies only: never read a body file from disk, check --body, -f body=, git commit -m, and --body-file - with a here-document, and keep the footer and em-dash rules in prose for a body posted from a file (rejected: read a body file only when gh or git commit -F is alone on the line; keep the gh post checks as prose entirely; drop the quest). Reason: the quest's Plan says a check that cannot stay narrow stays prose. The per-line cd and write tracking went with it.
  • Review fixes after the first pick: git push --tags origin feature now needs its refspec's : (only a tags-only push is exempt); mktemp honors a TMPDIR= assignment in front of it; (moq-dev/quest#553 review). Refuted: keeping the checked prose in the hook's context, since the quest's Goal removes it.
  • Merging main's quest setup --worktrees: the worktree hooks go only into Claude Code's settings; HookEntry carries a command beside its handler settings, which hold the SessionEnd timeout.

Waiting

  • Maintainer: design checkpoint after review round 3 of the third session (scores 7, 8, 8; head c25255e). Inline-only body checking landed as picked, and no finding since touches it. Every finding in the three rounds but the footer anchoring (fixed) comes from per-command option and path semantics, such as mktemp modes, $TMPDIR, and git push --repo (all fixed), and the three still open: where cp/mv/ln/rsync/touch put their destination in the /tmp check (cp -t <dir>, cp -t.tmp, rsync -t, touch -r/--reference, ln -s <target> with an implicit destination), and where git accepts a help flag (git rebase --interactive -h). Each fix exposes the next variant. Options:
    1. Narrow both checks (recommended): drop cp/mv/ln/rsync/touch from the /tmp check, keeping redirections, tee, mkdir, mktemp, git clone/worktree add, and Write/Edit/apply_patch paths, where the habitual mistake (logs and bodies written to /tmp) happens. Pass any git command with -h/--help anywhere in it, accepting the rare false pass of git push -o --help, since a missed nudge is not a block. This removes the class of findings and some code, following the quest's rule that a check which cannot stay narrow stays prose.
    2. Continue: parse attached -t<dir>, ln's one-operand form, and help flags after other options, keeping per-command grammars.
    3. Merge as is (after c25255e), accepting rare false denials whose message names the rule.
    4. Narrow the /tmp check further, to redirections and Write/Edit/apply_patch paths only.
      Check: a Design change: PR comment records the pick, then /quest-iterate applies it and reviews with quest review --new.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort
Implementer: Claude Code, claude-opus-5-5, low effort (iterate, M)

Suggested follow-ups

None worth a quest.

(Written by Claude Opus 5.5)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, high effort
  • Session: 01a1b9c4-426c-7703-878b-1c76630de27c, round 3
  • Head: c25255e, named in full once a round reports no findings
  • Base: origin/main at 19c6602
  • Score: 8/10, All four reported cases are fixed, but destination and help-option parsing still rejects several compliant commands.
  • Findings, open until the next round settles them:
    • Medium, src/cli/rules.ts:208: [P2] Recognize attached target-directory arguments
    • Medium, src/cli/rules.ts:209: [P2] Handle ln's implicit destination
    • Medium, src/cli/rules.ts:249: [P2] Recognize actual help flags after other options
  • The last round's findings, at 31a083b:
    • Fixed: medium, src/cli/rules.ts:204: [P2] Recognize long reference options before classifying operands; The valued-option set at src/cli/rules.ts:204 now includes --reference, and the previously denied reference-input example passes.
    • Fixed: medium, src/cli/rules.ts:207: [P2] Keep rsync option semantics separate from cp; The command-specific branch at src/cli/rules.ts:208 excludes rsync from target-directory parsing, correctly handling both reproduced rsync destinations.
    • Fixed: medium, src/cli/rules.ts:248: [P2] Distinguish help flags from option values; The narrowed help check at src/cli/rules.ts:249 no longer exempts `git push -o --help origin main`, which is correctly denied.
    • Fixed: low, src/cli/rules.ts:406: [P3] Anchor attribution to the complete body's end; The regex at src/cli/rules.ts:407 rejects content after attribution while allowing a final attribution line followed by whitespace.

@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

quest setup installs a PreToolUse hook for Claude Code and Codex that denies
tool calls breaking the rules a program can check: em dashes an agent adds,
a git push without explicit refspecs, a force-push or rebase, a new GitHub
post without its footer, and scratch writes under /tmp. Their prose leaves
the hook's context and stays in `quest context` for agents without it.

quest setup also writes Codex's SessionStart rules hook to hooks.json, with
an additionalContextLimit that keeps the rules whole.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Mechanical rules become checks feat(hooks): enforce the mechanical rules with PreToolUse hooks Nov 7, 2026
kixelated and others added 2 commits November 7, 2026 13:59
…d shells, check only clone destinations

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…separate git dir

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: Narrow the body sources. Drop the in-command file tracking (the written map): a body file that the same command line writes reads as unknown and passes, while --body-file - with a here-document stays checked directly. The maintainer picked this at the round-3 design checkpoint through /quest-unblock on 2026-11-08 (rejected: keep in-command tracking and mark expanded writes as unknown; merge as is; drop the footer and em-dash checks on gh commands).

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 7, 2026 20:47
…rule-checks

# Conflicts:
#	README.md
#	quest/m0/token-waste/README.md
#	src/cli/hooks.ts
#	src/cli/main.ts
#	src/runner/prompt.ts
#	test/hooks.test.ts
Drops the in-command file tracking that read a here-document's text as the
body file's contents. A file an earlier command on the line writes now reads
as unknown and passes; `--body-file -` with a here-document stays checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 8, 2026 04:56
@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1cbfdb1e-c038-4366-bdd3-c280c2ed6760
Commits

Reviewing files that changed from the base of the PR and between 738bbde and a0d8c95.

Files selected for processing: 12

  • README.md
  • SETUP.md
  • assets/CONTEXT.md
  • quest/m0/token-waste/README.md
  • quest/m0/token-waste/rule-checks.md
  • src/cli/hooks.ts
  • src/cli/main.ts
  • src/cli/rules.ts
  • src/cli/shell.ts
  • src/runner/prompt.ts
  • test/hooks.test.ts
  • test/rules.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Head: e100158
Base: 738bbde

[P2] Do not inspect relative body files against a stale working directory (src/cli/rules.ts:203). shell() keeps the hook's original cwd for every simple command and does not handle cd. For example, if body.md in the initial directory lacks a footer but .tmp/body.md contains a valid post with its footer, cd .tmp && gh pr comment 5 --body-file body.md reads the former and is denied even though gh would read the latter. The same issue affects stdin redirection. This contradicts the narrow, fail-open behavior: an unrelated file can block a legitimate post. Track directory changes where they are known, or treat relative body sources as unknown after a command changes the directory. Add a regression covering two same-named files with different contents and a preceding cd.

Direction: fix this false denial before landing; the chosen policy of letting unknown body sources pass does not require broader shell emulation.

Static review of the 12-file diff, tests, and recorded design decision only. No shell commands, tests, builds, or live hook checks were run; runtime behavior and CI success are not certified.

(Written by review (OpenAI))

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as draft November 8, 2026 05:12
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: Check inline bodies only. Stop reading body files from disk: check only --body, --body-file - with a here-document, and git commit -m, the pattern the rules already ask for. A body file posted from disk goes unchecked, and the footer and em-dash rules stay in prose for that case. The maintainer picked this at the second round-3 design checkpoint through /quest-unblock on 2026-11-08, following the quest's own rule that a check which can't stay narrow stays prose (rejected: read a body file only when gh or git commit -F is alone on the line; keep the gh post checks as prose entirely; drop the quest).

(Written by Claude Opus 5.5)

kixelated and others added 3 commits November 7, 2026 22:06
The maintainer's pick at the second round-3 design checkpoint: a body file
may hold something else by the time the line posts it, so the hook checks
only --body, body= fields, git commit -m, and --body-file - with a
here-document. The per-line cd and write tracking goes with it, and the
footer and em-dash rules stay in prose for bodies posted from a file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…refspecs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…target options

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant